Skip to content

fix: skip metadata-only SSE chunks instead of aborting stream in StreamingResponseAggregator#783

Closed
anishesg wants to merge 1 commit into
google:mainfrom
proudhare:fix/ph-issue-782
Closed

fix: skip metadata-only SSE chunks instead of aborting stream in StreamingResponseAggregator#783
anishesg wants to merge 1 commit into
google:mainfrom
proudhare:fix/ph-issue-782

Conversation

@anishesg
Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

Closes: #782

Vertex AI occasionally emits a leading SSE chunk that contains only response metadata (createTime, modelVersion, usageMetadata, etc.) with no Candidates field. This happens consistently with gemini-3-flash-preview + googleSearch grounding and caused roughly 40–50% of streaming calls to fail with "empty response", even though the actual content arrived in subsequent chunks.

The root cause is in internal/llminternal/stream_aggregator.go in ProcessResponse: when len(genResp.Candidates) == 0, the function called yield(nil, fmt.Errorf("empty response")) and returned, aborting the iterator. The fix changes this to a silent return, allowing the aggregator to skip metadata-only chunks and continue processing subsequent chunks that carry real content. This matches the behavior of adk-python's PROGRESSIVE_SSE_STREAMING path which handles identical chunks gracefully.

The unused fmt import was also removed as a consequence.

Testing Plan

Unit Tests

  • Added TestMetadataOnlyChunkDoesNotAbortStream in stream_aggregator_test.go that feeds a metadata-only chunk (no Candidates) followed by a real content chunk and asserts the aggregated response contains the expected text without any error.
  • All existing stream aggregator unit tests continue to pass.
go test ./internal/llminternal/... -run "TestProgressiveSSE|TestStreaming|TestFinishReason|TestMetadata" -v
--- PASS: TestProgressiveSSEStreamingFunctionCallArguments (0.00s)
--- PASS: TestProgressiveSSEPreservesThoughtSignature (0.00s)
--- PASS: TestProgressiveSSEHandlesEmptyFunctionCall (0.00s)
--- PASS: TestStreamingFCChunkWithWillContinueButNoPartialArgs (0.00s)
--- PASS: TestProgressiveSSEStreamingFunctionCalls (0.00s)
--- PASS: TestProgressiveSSEPreservesPartOrdering (0.00s)
--- PASS: TestMetadataOnlyChunkDoesNotAbortStream (0.00s)
--- PASS: TestFinishReasonUnexpectedToolCallPreservesErrorCode (0.00s)
PASS

Manual E2E Tests

The reproduction script from the issue (10 runs of gemini-3-flash-preview + googleSearch with StreamingModeSSE) now produces 10/10 successes with this patch applied, matching the behavior of StreamingModeNone and adk-python.

Checklist

  • I have read the CONTRIBUTING.md file.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added unit tests that prove my fix is effective or that my feature works.
  • All unit tests pass locally.
  • I have completed manual E2E testing.
  • Any dependent changes have been merged and published.

Fixes #782

…amingResponseAggregator

## Link to Issue or Description of Change

Signed-off-by: anish k <ak8686@princeton.edu>
Copy link
Copy Markdown
Collaborator

@kdroste-google kdroste-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anishesg, thank you for your contribution!
The fix seems OK.
Please see other comments for minor remarks

Thank you!

if len(genResp.Candidates) == 0 {
// shouldn't happen?
yield(nil, fmt.Errorf("empty response"))
// Vertex AI occasionally emits leading SSE chunks that contain only
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the information that sometimes SSE stream can start with an empty text parts mixed with chunks which do not contain any candidates.

}
}

func TestMetadataOnlyChunkDoesNotAbortStream(t *testing.T) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for the following sequence
Empty text
No candidates
No candidates
Empty text
No candidates
Non-empty text

@kdroste-google
Copy link
Copy Markdown
Collaborator

I'm closing this PR, the #918 is a replacement

@kdroste-google
Copy link
Copy Markdown
Collaborator

No longer applicable

@kdroste-google
Copy link
Copy Markdown
Collaborator

@anishesg , thank you!

@wolo-lab
Copy link
Copy Markdown

Thanks for jumping on this so quickly, @anishesg — the diagnosis here was
spot on, and the unit test made the behavior easy to reason about.

We ended up merging #918, which builds directly on your fix. The one change:
instead of dropping the metadata-only chunks, it passes them downstream, since
they carry usageMetadata we want to keep.

The fix shipped in v1.4.0. Thanks again for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streaming SSE aggregator aborts on metadata-only chunks

4 participants